-
Notifications
You must be signed in to change notification settings - Fork 7
Conversation
@pavlovcik check this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good let's finish QA expediently! @web4er is trying on the telegram side!
Co-authored-by: アレクサンダー.eth <[email protected]>
Co-authored-by: アレクサンダー.eth <[email protected]>
Co-authored-by: アレクサンダー.eth <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The issue says:
We must be able to associate which Telegram channel/topic goes to which GitHub repository.
It is working with group and topic, but doesn't work with channel.
-
This is unnecessary now because database is being used for this. It should be removed. https://github.com/ubiquity/telegram-ubiquibot/blob/2a57f4034a3e39b39944403384901586166af766/src/constants.ts#L4
-
supabase cli is not included with it in packages.json. Last time I check supabase cli doesn't work globally. So it should be included with it. And can we follow the exact structure of ubiquibot for supabase so that it is easier for the team to catch up. https://github.com/ubiquity/ubiquibot/tree/development/supabase
-
I think the SQL migration files were created manually. It should be generated using supabase cli to use sqls in the correct order.
-
You should push the updated readme with this PR.
Channel support added @web4er, Reinstall bot with post and edit message permission |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think supabase/config.toml file is not needed here.
Edit: This file is needed for local run, so keep it.
Co-authored-by: Maxima <[email protected]>
Co-authored-by: Maxima <[email protected]>
make sure variables are changed to the new names on cloudflare as well and filled accordingly |
|
|
Do I have to add it manually? because I did |
I think your code is not updated, because the variable names have changed so that should change automatically too. You can try deleting everything, updating your branch and running setup-key again |
We have to be done with this soon, its been open for too long.. We have #17 |
Yes, I thought it was missing that in code. Working now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, working well in group, topics, and channels.
I believe the following would be the steps to deploy this PR.
- create supabase db and deploy migrations
- set up environment.json, update new and old both vars, because names have changed.
- yarn setup-key
- yarn deploy
- add bot to group, and chat with bot using /start to provide github repo link.
These are handled automatically by the Github actions, Just some variable names change in secret |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like my earlier requested changes were not implemented please confirm the status and then I can approve and merge.
Perhaps they were on the other pull request that this one is based off of?
messages: [ | ||
{ | ||
role: "system", | ||
content: TRAINING, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you never renamed this to PROMPT_SYSTEM
|
||
export const handleFirstMenu = async (value: string, chatId: number, messageId: number, groupData: string) => { | ||
switch (value) { | ||
case "link_github": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall asking about a /register
command but I could be confused here.
const userContext = getUserSession(chatId); | ||
|
||
switch (userContext.v) { | ||
case "link_github": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/register
here as well.
"*`[]()" | ||
); | ||
const msg = data.html_url | ||
? `*Issue created: [Check it out here](${data.html_url})* with time estimate *${timeEstimate}*${assignees ? ` and @${tagged} as assignee` : ""}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also believe I wrote somewhere an adjusted message without the "check it out here" part but I have to look for that and it's a bit slow from my phone.
These changes were made in #17 |
You made the review and it got implemented here: c476f88 |
@pavlovcik can you add |
and |
You should have access to manipulate the secrets in this repository. I assume you can handle everything except for the Telegram bot token (but I just checked, and this is already in our secrets as of two weeks ago.) |
I only have access to cloudflare variables |
I updated your permissions to |
No, its a 404 page |
I sent you dm on telegram |
Resolves #3